Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove support for IMPLOT_INSTANTIATE_ALL_NUMERIC_TYPES #402

Merged
merged 1 commit into from
Sep 14, 2022

Conversation

pthom
Copy link
Contributor

@pthom pthom commented Sep 14, 2022

My two cents: given full configurability (looks great!) i would suggest not adding “ IMPLOT_INSTANTIATE_ALL_NUMERIC_TYPES. For those rare types it seems reasonable to simply let the user express their needs explicitly.”

I do agree, and this PR removes them.


especially as it may be tempting for users and we don’t know what long double would carry in term of math runtimes.

That is an interesting question! I made a quick study, and my findings are:

  • the difference of speed between float and double can be as high as 20% (and the result do depend on the arch and compiler)
  • when double and long double have the same size (8 bytes), long double is 20% to 30% slower (except on apple M1 processor)
  • when sizeof(long double) = 16 vs sizeof(double) = 8, long double can be 10 to 30 times slower (this depends on the compiler and platform)

I haven’t poked in internals but wondered if small types <32 or <64 could somehow attempt to reuse larger types functions, with special adapters? Intuitively it seems like u8 u16 could use the code for u64. This is implying that generated code size and link size could be reduced. With custom type list this is now perhaps a little harder to setup, so perhaps it would need an hardcoded proof of concept because getting it to work with custom type lists.

There is perhaps a solution, but I suspect it would be very likely be "platform / compiler / arch / type" specific and maybe difficult to maintain.

In the case of unsigned integers, a possible solution would be to:

  • implement only the full code for the u64 version
  • have a template function for u8, u16, u32 that would call the u64 version by first reading elements as u64.
  • have a way to read u64 values inside a u8, u16 or u32 array. A possible solution would be to set u64 value = 0;, then fill the right portion of value's bytes with the bytes from the array

However, based on my observations, the memory layout for a 64 bits int (on a Mac M1) is a mix of:

  • little-endian (for bits)
  • big-endian (between bytes)
  • big-endian (between groups of 4 bytes)
    And I suspect that this is very much platform dependent.

Example, if I set
int64_t v64 = 0x123456789ABCDE00;
and examine the memory at &v64, it looks like this on my Mac:

    00 de bc 9a   78 56 34 12

Only use IMPLOT_CUSTOM_NUMERIC_TYPES

- Added doc on how to use all types in `implot_items.cpp` doc
- Updated FAQ
- CI's CMakeLists now enables to fully customize the type
  via -DIMPLOT_CUSTOM_NUMERIC_TYPES="(float)(double)" for example
@epezent epezent merged commit 98c76ed into epezent:master Sep 14, 2022
pthom added a commit to pthom/imgui_bundle that referenced this pull request Sep 20, 2022
See merged implot PRs epezent/implot#402 and epezent/implot#397 which were provided during the development of these bindings
Ben1138 pushed a commit to Ben1138/implot that referenced this pull request Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants